Revision of qcscore_model_spe branch - #64

This is a wrong suggestion. model_formula should be built ALWAYS using the computed variables contained in out_var, not just when modelFormula is null. out_var is the vector of metrics resulting from the checks made by .prepQCContext. If the number of outliers is not enough, that metric is automatically discarded from the vector

non è così, se fornisci un argomento (modelFormula), stai dando la possibilità di usarlo.
Altrimenti è inutile fornire tale argomento.

Quindi il funzionamento logico è:
se la formula in argomento è NULL, allora si usano le componenti che abbiamo impostato noi di default. Quindi questo è il comportamento standard suggerito dagli autori (noi).
In alternativa, se l'utente vuole sperimentare altre metriche, scrive una formula con queste metriche e la passa a modelFormula, che quindi andrà a sostituire la formula decisa da noi.

Ma l'argomento infatti viene utilizzato! Viene usato in questa parte qui, per costruire la prima versione della formula custom che poi però deve essere controllata:

if (!is.null(modelFormula)) { model_formula <- modelFormula metricList <- attr(terms(as.formula(modelFormula)), "term.labels") metricList <- metricList[!grepl(":", metricList, fixed=TRUE)] if("I(abs(log2AspectRatio) * as.numeric(dist_border < 50))" %in% metricList) { metricList <- gsub("I\\(abs\\((log2AspectRatio)\\) \\* as\\.numeric\\((dist_border) < 50\\)\\)", "log2AspectRatio", metricList) } }

L'utente può scegliere che metriche mettere nella formula, ma non può mettere quello che vuole. Devono essere tra le metriche che noi abbiamo approfondito, come abbiamo illustrato nella supplementary note. Altrimenti che senso ha aver inserito una tabella di metriche e tutta la spiegazione sul perché le abbiamo scelte? Se avessimo dovuto lasciare all'utente il compito di cercarsi quali metriche combinare assieme, non avrebbe avuto senso pubblicare proprio SpaceTrooper.

model_formula <- getModelFormula(names(out_var))

Questa riga di codice ci deve essere sempre perché viene dopo questo:

ctx <- .prepQCContext(spe, metricList, verbose) df <- ctx$df; out_var <- ctx$out_var; tech <- ctx$tech

Il che significa che stiamo veramente dando all'utente la possibilità di togliere o mettere delle metriche nella sua formula, ma ci stiamo anche assicurando che la formula che usa rispetti i criteri che valgono per la formula di default e che devono valere sempre. Ossia:

1. log2SignalDensity deve esserci per forza perché è il requisito minimo per calcolare il Quality Score;
2. per ciascuna metrica ci deve essere un numero minimo di outlier pari allo 0.01% della numerosità del dataset;
3. per dataset di Xenium e MERFISH/MERSCOPE non si deve usare il border effect.

Se non rispettiamo il primo criterio, non abbiamo la più pallida idea di quello che si ritrova in mano l'utente per fare QC, e mettere una metrica per cui non c'è un numero sufficiente di outlier, o il border effect per tecnologie in cui non lo abbiamo osservato, vuol dire addestrare il modello su una variabile che non è informativa per quel dataset, col rischio di andare a filtrare delle cellule su un criterio che è sbagliato.

ok, capisco quello che dici.

A questo punto, dobbiamo però separare chiaramente libertà dell’utente e validazione del metodo.

Secondo me qui vanno distinti due problemi diversi.

Il primo è permettere davvero all’utente di usare una formula personalizzata. Se esiste un argomento chiamato modelFormula, la formula fornita deve essere quella effettivamente usata dal modello. Se l’utente sceglie una formula poco sensata, se ne assume la responsabilità; al massimo possiamo avvertirlo, ma non dovremmo sostituirla silenziosamente con una formula ricostruita da noi.

Con il codice attuale, invece, modelFormula viene usata soltanto per estrarre le metriche e poi viene sovrascritta da:

model_formula <- getModelFormula(names(out_var))

Quindi l’utente non sta realmente scegliendo la formula, ma soltanto un sottoinsieme di variabili e combinato nel modo stabilito da noi (ad es imponendo che ci siano sempre gli interaction term).

Il secondo problema è fare dei controlli di validità. Questo è indipendente dal primo e può essere gestito separatamente verificando, per esempio:

• che le variabili indicate esistano in colData;
• che log2SignalDensity sia presente, se la consideriamo un requisito strutturale del QScore;
• che alcune metriche siano compatibili con la tecnologia;
• che le metriche validate da SpaceTrooper abbiano un numero sufficiente di outlier.

Le metriche fuori dalla whitelist potrebbero comunque essere accettate, producendo un warning:

unsupported <- setdiff(
    all.vars(as.formula(modelFormula)),
    validated_metrics
)

if (length(unsupported)) {
    warning(
        "The following predictors have not been validated by SpaceTrooper ",
        "and are used at the user's own risk: ",
        paste(unsupported, collapse = ", ")
    )
}

La scelta della formula dovrebbe quindi restare distinta dai controlli:

model_formula <- if (is.null(modelFormula)) {
    getModelFormula(names(out_var))
} else {
    modelFormula
}

In sintesi: noi possiamo controllare che la formula sia applicabile e avvertire l’utente quando esce dal perimetro validato, ma non dovremmo modificarne automaticamente la struttura. Altrimenti modelFormula non è davvero una formula custom.

@BenedettaBanzi

@BenedettaBanzi

@BenedettaBanzi never do the version bump together with all the modifications.
It requires a different commit.

ok, capisco quello che dici.

A questo punto, dobbiamo però separare chiaramente libertà dell’utente e validazione del metodo.

Secondo me qui vanno distinti due problemi diversi.

Il primo è permettere davvero all’utente di usare una formula personalizzata. Se esiste un argomento chiamato modelFormula, la formula fornita deve essere quella effettivamente usata dal modello. Se l’utente sceglie una formula poco sensata, se ne assume la responsabilità; al massimo possiamo avvertirlo, ma non dovremmo sostituirla silenziosamente con una formula ricostruita da noi.

Con il codice attuale, invece, modelFormula viene usata soltanto per estrarre le metriche e poi viene sovrascritta da:

model_formula <- getModelFormula(names(out_var))

Quindi l’utente non sta realmente scegliendo la formula, ma soltanto un sottoinsieme di variabili e combinato nel modo stabilito da noi (ad es imponendo che ci siano sempre gli interaction term).

Il secondo problema è fare dei controlli di validità. Questo è indipendente dal primo e può essere gestito separatamente verificando, per esempio:

• che le variabili indicate esistano in colData;
• che log2SignalDensity sia presente, se la consideriamo un requisito strutturale del QScore;
• che alcune metriche siano compatibili con la tecnologia;
• che le metriche validate da SpaceTrooper abbiano un numero sufficiente di outlier.

Le metriche fuori dalla whitelist potrebbero comunque essere accettate, producendo un warning:

unsupported <- setdiff(
    all.vars(as.formula(modelFormula)),
    validated_metrics
)

if (length(unsupported)) {
    warning(
        "The following predictors have not been validated by SpaceTrooper ",
        "and are used at the user's own risk: ",
        paste(unsupported, collapse = ", ")
    )
}

La scelta della formula dovrebbe quindi restare distinta dai controlli:

model_formula <- if (is.null(modelFormula)) {
    getModelFormula(names(out_var))
} else {
    modelFormula
}

In sintesi: noi possiamo controllare che la formula sia applicabile e avvertire l’utente quando esce dal perimetro validato, ma non dovremmo modificarne automaticamente la struttura. Altrimenti modelFormula non è davvero una formula custom.

@BenedettaBanzi come mai hai cambiato qui?
ogni cambio di argomento diventa un "deprecated"